Skip to content

Conversation

@bfirsh
Copy link
Contributor

@bfirsh bfirsh commented Jan 21, 2013

... so they can be used for testing custom types. See #156

@wmertens
Copy link
Contributor

I have no strong opinion - @josephg looks ok?

@bfirsh
Copy link
Contributor Author

bfirsh commented Apr 3, 2013

I've also included types.helpers here so bootstrapTransform can be used on the server. (It's only currently available in the browser as sharejs._bt).

@josephg
Copy link
Owner

josephg commented Apr 14, 2013

I'm confused - why is randomWord imported in each type's tests?

@josephg
Copy link
Owner

josephg commented Apr 14, 2013

This patch confuses me.

  • Most of the changes have nothing to do with exposing test helpers to external projects.
  • bootstrapTransform is already used by the server (the JSON and old text types use it).
  • Its bad form to expose test/ paths from the src/ directory. Do it at the top level instead, in src/index.coffee.

And also, the OT types have moved into their own project. Once you're happy with it, this pull request will need to be duplicated to josephg/ot-types .

@bfirsh
Copy link
Contributor Author

bfirsh commented Apr 15, 2013

Ahah – I didn't see the ot-types project before. That's pretty cool.

I moved randomWord into test/helpers/misc just for the sake of organising it. Would it be better in a separate file?

Would you prefer everything to be exposed as require('share').test?

@josephg
Copy link
Owner

josephg commented Apr 15, 2013

Yeah require('share').test sounds good. I moved randomWord in with the rest of the randomizer code in ot-types because you're right, it makes more sense like that.

Moving forward, I'm not really sure how these projects should be hooked up. I guess the ot types project should expose the randomizer and bootstrapTransform functions directly. If people want to use them from sharejs, they can just require('ot-types') directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants